-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove the limit on Gunicorn dependency #15611
Conversation
It seems that the < 20.0 limit for gunicorn was added at some point in time without actual reason. We are already using gunicorn in 1.10 line of Airflow, so it should not be a problem to bump the version of gunicorn, especially that the 19. line is somewhat deprecated already. This change came after the discussion n apache#15570
6baf862
to
a29b248
Compare
Looks like all tests are passing with gunicorn 20.1.0 (the latest one). I tested it locally as well, and everything seems to be fine Airflow works with several gunicorn workers - looks like no problems. I think we can safely merge this one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel great about no upper limit here - it means Gunicorn could release a v22 that breaks us.
That's when the constraints files is useful 🙂 Python packaging encourages the "compatible unless proven guilty" approach since it's much easier for the user to add extra constraints if gunicorn 22 proves incompatible, but it's much more difficult to remove |
Agree with @uranusjr - our constraints approach is designed to prevent precisely this problem and it works with most of our dependencies. |
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
It seems that the < 20.0 limit for gunicorn was added at some point in time without actual reason. We are already using gunicorn in 1.10 line of Airflow, so it should not be a problem to bump the version of gunicorn, especially that the 19. line is somewhat deprecated already. This change came after the discussion n apache#15570 (cherry picked from commit d7a14a8)
It seems that the < 20.0 limit for gunicorn was added at some point
in time without actual reason. We are already using gunicorn in
1.10 line of Airflow, so it should not be a problem to bump the
version of gunicorn, especially that the 19. line is somewhat
deprecated already.
This change came after the discussion n #15570
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.